Skip to content

Fix ClipQuantFusion crash when Clip has multiple input edges #26923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
qti-yuduo wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix ClipQuantFusion crash when Clip has multiple input edges #26923

qti-yuduo wants to merge 1 commit into from
+36 −1

Conversation

@qti-yuduo
Copy link
Contributor

@qti-yuduo qti-yuduo commented Jan 7, 2026

Motivation

ClipQuantFusion in clip_quantizelinear.cc calls graph_utils::RemoveNode() without first checking graph_utils::CanRemoveNode(). When a Clip node has min/max inputs from DequantizeLinear nodes (instead of initializers), it has multiple input edges. RemoveNode() throws exception:

 [ONNXRuntimeError] : 6 : RUNTIME_EXCEPTION : Exception during initialization: graph_utils.cc:650 bool onnxruntime::graph_utils::RemoveNode(onnxruntime::Graph&, onnxruntime::Node&) Should be unreachable if CanRemoveNodeAndMergeEdges is in sync with the logic here.

Fix:

Added CanRemoveNode() check to ClipQuantFusion::SatisfyCondition() to skip nodes that cannot be safely removed.

Test:

Added ClipQuantFusion_MultipleInputEdges test that creates a Clip node with min from a DQ node (2 input edges) and verifies the optimizer doesn't crash.

@qti-yuduo qti-yuduo changed the title [QNN EP] Fix ClipQuantFusion crash when Clip has multiple input edges Fix ClipQuantFusion crash when Clip has multiple input edges Jan 7, 2026
@yuslepukhin yuslepukhin requested a review from Copilot January 13, 2026 18:58
Copilot AI reviewed Jan 13, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

onnxruntime/core/optimizer/qdq_transformer/clip_quantizelinear.cc
Comment on lines +98 to +100
if (!graph_utils::CanRemoveNode(graph, node, logger)) {
return false;
}
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CanRemoveNode check should ideally be performed earlier in the SatisfyCondition method, before checking the next_node. This ordering is more efficient because if the current node cannot be removed, there's no need to validate the following QuantizeLinear node. Consider moving this check to after line 89 (after the initial node validation) and before line 92 (before the next_node checks).

Copilot uses AI. Check for mistakes.
@tianleiwu
Copy link
Contributor

tianleiwu commented Jan 13, 2026

From AI

Summary

This PR fixes a runtime exception in the ClipQuantFusion optimizer. The crash occurred because the code attempted to remove a Clip node without verifying if it was safe to do so, specifically when the node had multiple input edges (e.g., dynamic min/max inputs from DequantizeLinear nodes).

Key Changes

  • Fix: Added a call to graph_utils::CanRemoveNode() within ClipQuantFusion::SatisfyCondition().
  • Test: Added a new test case ClipQuantFusion_MultipleInputEdges to reproduce the scenario and verify the fix.

Review Analysis

Correctness

  • Pattern: It is a standard and necessary practice in ONNX Runtime graph transformations to check CanRemoveNode() before calling RemoveNode(). The fix correctly implements this missing check.
  • Root Cause: The description clearly explains why the crash happened (multiple input edges causing RemoveNode to fail), and the fix directly addresses it.

Performance

  • Impact: Negligible. The added check is lightweight and prevents a crash, which is infinitely better than failing.

Conclusion

This is a straightforward and correct bug fix. The inclusion of a regression test gives high confidence in the solution. LGTM.

@yuslepukhin
Copy link
Member

yuslepukhin commented Jan 13, 2026

/azp run Linux QNN CI Pipeline,Win_TRT_Minimal_CUDA_Test_CI,Windows ARM64 QNN CI Pipeline,Windows GPU Doc Gen CI Pipeline

@azure-pipelines
Copy link

azure-pipelines bot commented Jan 13, 2026

Azure Pipelines successfully started running 4 pipeline(s).

adrianlizarraga
adrianlizarraga approved these changes Jan 14, 2026
View reviewed changes
Copy link
Contributor

@adrianlizarraga adrianlizarraga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@adrianlizarraga adrianlizarraga adrianlizarraga approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@qti-yuduo @tianleiwu @yuslepukhin @adrianlizarraga